-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
deps: update to golang v1.21 #96
deps: update to golang v1.21 #96
Conversation
Hi @fmartingr
|
Not that I am aware of, I'd say let's use the same in all projects so results are the same.
I'm assuming they used it that way to avoid memory allocation as the comment suggest, never did something like that. We may need to add some benchmarks, but I wouldn't worry too much about that for now. Now that we are on it, I think we can bump golang to 1.21 since 1.22 will release in February and 1.20 will become obsolete. |
i review most of the workflows and remove what i think we don't need. side-note: we have some PR from dependencies not bad if you can review them. |
Why did you remove all linters and its configuration files from the workflows? You only left golangci-lint but removed both configuration files for it. Maybe I expressed myself wrong, with my previous comment I meant to use the same yaml file for golangci-lint, but I think we should leave the other linters as well unless you have found they are not useful to this project. But they seem useful to me at a first glance (all but the shellcheck, since there does not seem to be shell files in the repo). What do you think? |
Also, update the dependabot.yml file with the same we have in shiori: https://github.com/go-shiori/shiori/blob/master/.github/dependabot.yml This will make it group all updates in the same PR, which is easier to merge than a bunch of separate ones :) |
I prefer the way we do golangci instead of being limited to the specific test, so I remove config of golangci.
So I try to keep things we have in |
Yeah, sorry, I was referring to golangci-lint specifically. Please revert the changes for all linters but the shellcheck one, since I'm not aware of any bash/sh code in this repository. |
@fmartingr |
I think it is ready now 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, left you some things around.
Also i would bring back the golangci.yml
file since it has specific configuration that may be there for some reason, until we figure out what we need and what dont. Is there any speciifc linter or configuration causing false positives or something like that that I should be aware of?
in general i try to we have same settings in all shiori projects + specific needs for specific project Things we don't need in the old config and fix in the default config.
Exclude things in old config that we have in the default config.
what do you think? you can read more about that here
Specific settings in the old config that I think most of them are not necessary
I think it is not a big deal that we do other Shiori project.(if you want I can check the project for error handling in other PR too). should i return that?
What do you think? based on this which part you want to keep? |
Don't be sorry, thanks a lot for your thorough investigation into this. Let's go with your changes since all points you mention seems fair to me. We can always change it later if we think it's necessary anyway. Awesome work! |
Co-authored-by: Felipe Martin <[email protected]>
try to update golang v1.14 to 1.21
io/ioutil